Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[R-package] fix R examples and lgb.plot.interpretation() #2999

Closed
wants to merge 2 commits into from
Closed

[R-package] fix R examples and lgb.plot.interpretation() #2999

wants to merge 2 commits into from

Conversation

jameslamb
Copy link
Collaborator

See comments from @StrikerRUS

A few of the R examples are broken right now. These weren't caught by tests because one is only a warning, one is a plot that is generated successfully but is incorrect, and one is an issue specific to our readthedocs builds.

I think the issue for lgb.cv() is #2715. In that issue we found that data.table 1.11.4 and R 3.6.0 lead to a data.table error ending in column or argument 2 is NULL. I think this can be fixed by upgrading to the data.table 1.12.1 in conf.py.

To fix the lgb.set.categorical() example, I changed file names across several examples to be sure that no two examples write to the same file name.

image

This issue with lgb.plot.interpretation() is because I made a change and forgot to use abs().

image

new plot:

image

I was not able to reproduce the data.table error in #2989 (comment). It

@jameslamb jameslamb added the fix label Apr 16, 2020
@jameslamb jameslamb mentioned this pull request Apr 16, 2020
Merged
@StrikerRUS
Copy link
Collaborator

Thanks @jameslamb for taking a look!

I think the issue for lgb.cv() is #2715.

But we do not use lgb.dl() at RTD and R version is 3.5 there. Probably we should update our FAQ entry...

If you work from LightGBM's branch, not from your fork, then I'll be able to enable RTD builds for that branch and make sure that all issues are gone. Will it make sense?

@jameslamb
Copy link
Collaborator Author

Thanks @jameslamb for taking a look!

I think the issue for lgb.cv() is #2715.

But we do not use lgb.dl() at RTD and R version is 3.5 there. Probably we should update our FAQ entry...

If you work from LightGBM's branch, not from your fork, then I'll be able to enable RTD builds for that branch and make sure that all issues are gone. Will it make sense?

Sure! I can recreate the PR from a LightGBM branch.

@jameslamb jameslamb mentioned this pull request Apr 17, 2020
Merged
@jameslamb
Copy link
Collaborator Author

closing in favor of #3002

@jameslamb jameslamb closed this Apr 17, 2020
@jameslamb jameslamb deleted the fix/r-examples branch April 25, 2020 17:05
@lock lock bot locked as resolved and limited conversation to collaborators Jun 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Laurae2 Laurae2 Awaiting requested review from Laurae2

@guolinke guolinke Awaiting requested review from guolinke guolinke is a code owner

@StrikerRUS StrikerRUS Awaiting requested review from StrikerRUS StrikerRUS is a code owner

Assignees
No one assigned
Labels
fix
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jameslamb @StrikerRUS